fix(buzz-sdk): preserve self-referential p tags in add/remove member events (#4326) - #4380
Closed
iroiro147 wants to merge 2 commits into
Closed
fix(buzz-sdk): preserve self-referential p tags in add/remove member events (#4326)#4380iroiro147 wants to merge 2 commits into
iroiro147 wants to merge 2 commits into
Conversation
…tent -) (block#4361) Previously `messages edit --content -` stored the literal string "-" as the replacement message body, silently replacing the original message with a dash when an agent (or shell pipeline) attempted a multi-line edit. Now edit resolves the content flag through read_or_stdin, exactly like send (introduced in block#624), so `messages edit --content -` reads the replacement body from stdin before validation and signing. The command help text is updated to document the behavior. Adds a regression test pinning the resolution expression: a bare dash is never a publishable value after resolution. Fixes block#4361 Signed-off-by: Sarthak Singh <sarthak.singh@juspay.in>
…events (block#4326) In nostr 0.44, EventBuilder.sign_with_keys strips a p tag whose value equals the signer's own pubkey. This made every self-targeted channel-membership operation impossible to express: buzz channels remove-member --pubkey <your own key> => relay 400 invalid: missing p tag The builder correctly constructed both tags — the loss happened at signing. Fix: add .allow_self_tagging() to build_add_member (kind 9000) and build_remove_member (kind 9001), matching the existing pattern used by NIP-IA archive/unarchive events (KIND_IA_*) which face the same signer==target symmetry (PR block#2568). Regression tests confirm: signing with a Keys whose pubkey X matches the target_pubkey leaves the p tag on the wire (before the fix the event had only the h tag). 254/254 buzz-sdk tests pass. Fixes block#4326 Signed-off-by: Sarthak Singh <sarthak.singh@juspay.in>
Contributor
Author
|
Closing this in favour of #4338, which addresses the same issue (#4326) with a 45-line change to builders.rs. I had two open PRs against #4326 — that's my duplication, not something you should have to triage. Trimming my open-PR count on buzz so what's left is worth your time. No action needed from you. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A
ptag whose value equals the signing key's own pubkey is dropped between the SDK builder and the wire. The builder constructs it correctly; the signed event that reaches the relay does not contain it.This makes every self-targeted channel-membership operation impossible to express:
buzz channels remove-member --pubkey <your own key>→ relay replies400 invalid: missing p tagbuzz channels add-member --pubkey <your own key> --role <r>→ same shape (self-add is explicitly permitted relay-side)The relay is behaving correctly —
extract_p_tagreturnsNonebecause the event genuinely has noptag. The loss happens client-side.Impact: any actor removing itself from a channel via the CLI is blocked, which includes the legitimate "an agent/identity should relinquish standing capability in a channel it no longer belongs in" case that #2928 discusses.
Root cause
crates/buzz-sdk/src/builders.rsbuild_add_member(kind 9000) andbuild_remove_member(kind 9001) built both the["h", …]and["p", target]tags, butEventBuilder::sign_with_keysin nostr 0.44 strips a matchingptag before signing (it suppresses self-referential tagging by default). Other builders in the same file that share this signer==target symmetry (KIND_IA_*identity archive/unarchive, added in PR #2568) already applied.allow_self_tagging()to opt out of that strip — this path did not.Fix
Applied
.allow_self_tagging()to thebuild_add_memberandbuild_remove_memberreturn values, matching the existing NIP-IA pattern in the same crate. Two lines of production code; two new regression tests.What was verified
cargo check -p buzz-sdk— clean.cargo test -p buzz-sdk— 254/254 passing (252 baseline + 2 new regression tests).remove_member_self_tag_survives_signingandadd_member_self_tag_survives_signingsign withKeys::generate(), set the target to that same key's own pubkey hex, and assert the"p"tag survives on the wire (previously the event shipped with only the"h"tag).Notes
The issue reporter traced the loss site to
EventBuilder::sign_with_keyswith high confidence and suggested either preserving self-referentialptags through signing, or failing client-side. Preserving (mirroring the NIP-IA precedent) is the minimal, protocol-legitimate fix — NIP-29 explicitly permits a user to remove or re-add themselves.buzz channels leaveremains the simpler single-tag alternative but is self-only and does not cover the general "remove member X" case.Fixes #4326